Skip to content

Use send_help to ensure that our help command is correctly invoked#949

Merged
kwzrd merged 2 commits into
masterfrom
help-command-fix-invocation
May 17, 2020
Merged

Use send_help to ensure that our help command is correctly invoked#949
kwzrd merged 2 commits into
masterfrom
help-command-fix-invocation

Conversation

@SebastiaanZ
Copy link
Copy Markdown
Contributor

@SebastiaanZ SebastiaanZ commented May 17, 2020

Currently, we're invoking our help command manually like this:

await ctx.invoke(self.bot.get_command("help"), "bot")

However, unfortunately, this fails when the command callback function is decorated, leading to exceptions like:

AttributeError: 'Help' object has no attribute 'channel'
  File "discord/ext/commands/core.py", line 83, in wrapped
    ret = await coro(*args, **kwargs)
  File "bot/cogs/snekbox.py", line 292, in eval_command
    await ctx.invoke(self.bot.get_command("help"), "eval")
  File "discord/ext/commands/context.py", line 132, in invoke
    ret = await command.callback(*arguments, **kwargs)
  File "bot/decorators.py", line 149, in inner
    if ctx.channel.id == destination_channel:

Error executing command invoked by <>: !e

See Sentry issue: BOT-4T

The solution is to use the way of invoking the help command built built into discord.py: ctx.send_help, as the recent refactoring of the help command made our custom help command compatible with that Context method.

@SebastiaanZ SebastiaanZ added t: bug Something isn't working p: 0 - critical Needs to be addressed ASAP a: information Related to information commands: (doc, help, information, reddit, site, tags) labels May 17, 2020
@SebastiaanZ SebastiaanZ requested a review from a team as a code owner May 17, 2020 10:20
@SebastiaanZ SebastiaanZ requested review from aeros and ikuyarihS and removed request for a team May 17, 2020 10:20
@SebastiaanZ
Use send_help to invoke command help
4e24e9c 
After the refactoring of the help command, we need to use the built-in
method of calling the help command: `Context.send_help`. As an argument,
the qualified name (a string containing the full command path, including
parents) of the command can be passed.

Examples:

- await ctx.send_help("reminders edit")

This would send a help embed with information on `!reminders edit` to
the Context.

- await ctx.send_help(ctx.command.qualified_name)

This would extract the qualified name of the command, which is the full
command path, and send a help embed to Context.

- await ctx.send_help()

This will send the main "root" help embed to the Context.
@SebastiaanZ SebastiaanZ force-pushed the help-command-fix-invocation branch from f73489f to 4e24e9c Compare May 17, 2020 10:26
@mathsman5133
Copy link
Copy Markdown
Contributor

mathsman5133 commented May 17, 2020

A Quick comment, would it be more efficient to pass in ctx.command rather than the name of the command, where applicable? Passing in a string means an extra lookup and if for some reason the command name were to change this way means an additional change isn't required

@SebastiaanZ
Copy link
Copy Markdown
Contributor Author

SebastiaanZ commented May 17, 2020 •
edited
Loading

A Quick comment, would it be more efficient to pass in ctx.command rather than the name of the command, where applicable? Passing in a string means an extra lookup and if for some reason the command name were to change this way means an additional change isn't required

The docs don't mention support for passing in a Context object. It can either be a Cog-instance, a Command-instance, or a string.

@mathsman5133
Copy link
Copy Markdown
Contributor

mathsman5133 commented May 17, 2020 •
edited by SebastiaanZ
Loading

Right, but can't you get a Command object with ctx.command?

Probably, I'll take a look if that works for every case.

@SebastiaanZ
Use Command-object for send_help
87cec1a 
As @mathsman5133 pointed out, it's better to use the `Command`-instance
we typically already have in the current context than to rely on parsing
the qualified name again.

The invocation is now done as: `await ctx.send_help(ctx.command)`
@SebastiaanZ
Copy link
Copy Markdown
Contributor Author

SebastiaanZ commented May 17, 2020

I accidentally hit edit instead of reply.

lemonsaurus
lemonsaurus approved these changes May 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@lemonsaurus lemonsaurus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This change is an improvement anyway, the previous approach was fighting the framework.

kwzrd
kwzrd approved these changes May 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@kwzrd kwzrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appears functional and I agree that this is an overall improvement.

@kwzrd kwzrd merged commit 65b02cd into master May 17, 2020
@kwzrd kwzrd deleted the help-command-fix-invocation branch May 17, 2020 10:52
@SebastiaanZ SebastiaanZ mentioned this pull request May 17, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lemonsaurus lemonsaurus lemonsaurus approved these changes

@ikuyarihS ikuyarihS Awaiting requested review from ikuyarihS

@aeros aeros Awaiting requested review from aeros

+1 more reviewer

@kwzrd kwzrd kwzrd approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: information Related to information commands: (doc, help, information, reddit, site, tags) p: 0 - critical Needs to be addressed ASAP t: bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@SebastiaanZ @mathsman5133 @lemonsaurus @kwzrd